-
Notifications
You must be signed in to change notification settings - Fork 472
Fix permanent drift in databricks_model_serving when using plaintext credentials
#5125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Fixes #5074 The GET API for serving endpoints does not return sensitive *_plaintext credential fields for external models (e.g., openai_api_key_plaintext, google_service_account_key_plaintext). This causes Terraform to detect drift even when no actual changes have been made. Changes: - Added reflection-based copySensitiveFields() helper function that automatically copies all *Plaintext fields from Terraform state to API response during Read operations - Applied fix to both databricks_model_serving and databricks_model_serving_provisioned_throughput resources - Added comprehensive unit tests for the sensitive field copying logic - Refactored sensitive field tests to use MockWorkspaceClientFunc pattern The solution is generic and will automatically handle new external model providers and their plaintext credential fields without code changes.
rauchy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but wouldn't an explicit and concrete solution using SetSensitive simplify here?
--- a/serving/resource_model_serving_provisioned_throughput.go
+++ b/serving/resource_model_serving_provisioned_throughput.go
@@ -30,6 +30,19 @@ func ResourceModelServingProvisionedThroughput() common.Resource {
common.CustomizeSchemaPath(m, "ai_gateway", "guardrails", "input", "pii").SetOptional().SetComputed()
common.CustomizeSchemaPath(m, "ai_gateway", "guardrails", "input", "pii", "behavior").SetOptional().SetComputed()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "ai21labs_config", "ai21labs_api_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "amazon_bedrock_config", "aws_access_key_id_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "amazon_bedrock_config", "aws_secret_access_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "anthropic_config", "anthropic_api_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "cohere_config", "cohere_api_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "databricks_model_serving_config", "databricks_api_token_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "google_cloud_vertex_ai_config", "private_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "openai_config", "openai_api_key_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "openai_config", "microsoft_entra_client_secret_plaintext").SetSensitive()
+ common.CustomizeSchemaPath(m, "config", "served_entities", "external_model", "palm_config", "palm_api_key_plaintext").SetSensitive()
+|
@rauchy SetSensitive alone won't help - the value isn't returned by the backend, leading to setting it to empty string on Read |
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Changes
The GET API for serving endpoints does not return sensitive *_plaintext credential fields for external models (e.g., openai_api_key_plaintext, google_service_account_key_plaintext). This causes Terraform to detect drift even when no actual changes have been made.
Changes:
The solution is generic and will automatically handle new external model providers and their plaintext credential fields without code changes.
Fixes #5074
Tests
make testrun locallydocs/folderinternal/acceptanceNEXT_CHANGELOG.mdfile